-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add 'locate' endpoint to borrow API #9842
Add 'locate' endpoint to borrow API #9842
Conversation
Add a new endpoint to redirect to a worldcat page based on identifier data provided for a given edition. Prioritizes oclc, then isbn 13, isbn 10, and finally title.
if edition.get('oclc_numbers', None): | ||
redirect_url += f'title/{edition.oclc_numbers[0]}' | ||
elif edition.get('isbn_13', edition.get('isbn_10', None)): | ||
redirect_url += f'isbn/{edition.isbn_13[0] or edition.isbn_10[0]}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is problematic because in line 205 we check whether isbn13 or isbn10 is available... But consider the case where there is an isbn10 but not an isbn13
In line 206, even though there's no isbn13, we're still trying to access the 0th element which doesn't exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, /isbn converts to /search on worldcat which isn't ideal, but it is what it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, good catch!
Works nicely! https://testing.openlibrary.org/books/OL7826547M/A_Game_of_Thrones/borrow?action=locate Squashing & merging |
Closes #9829
Feature. Adds a new endpoint to redirect to a worldcat page based on identifier data provided for a given edition. Prioritizes oclc, then isbn 13, isbn 10, and finally title.
Technical
Adds an extra check to the
plugins/upstream/borrow.py
borrow class. If the endpoint /books/{olid}/{title}/borrow is reached with the parameteraction=locate
, a redirect URL is constructed and redirected to.Testing
/borrow?action=locate
to the end of the urlScreenshot
Stakeholders
@mekarpeles